A couple of small style cleanups. #12156
Conversation
|
@nabijaczleweli @behlendorf Continuing the conversation from #11993 |
In `zpool_load_compat()`:
* initialize `l_features[]` with a loop rather than a static
initializer.
* don't redefine system constants; use private names instead
Rationale here:
When an array is initialized using a static {foo}, only the specified
members are initialized to the provided values, the rest are
initialized to zero. While B_FALSE is of course zero, it feels
unsafe to rely on this being true forever, so I'm inclined to sacrifice
a few microseconds of runtime here and initialize using a loop.
When looking for the correct combination of system constants to use
(in open() and mmap()), I prefer to use private constants rather than
redefining system ones; due to the small chance that the system
ones might be referenced later in the file. So rather than defining
O_PATH and MAP_POPULATE, I use distinct constant names.
Signed-off-by: Colm Buckley <colm@tuatha.org>
|
@nabijaczleweli could you take a look? |
|
No opinion on this. The defines are stylistic, and the loop is by definition identical to the aggregate init, so it's just more noise for no good reason, but can't not yield the same codegen, so meh. |
With respect; they are not quite the same; |
My preference is not to redefine system constants in private source; this file is very long with dozens of contributors at least, and who knows what will eventually be added to it. The principle of least surprise would lead me to leave system constants like |
Consider also a future extension of this logic which might need an initially "all-true" array; following the static initializer pattern, a developer might be tempted to write |
|
Exactly – they're identical, as guaranteed by WG14. Similarly for the macros – I defined them as equivalent fallbacks, you prefer to define an explicit semantic variant. Sure, that's not how I'd write it in these particular cases, but the very fact that you did means you prefer it (for reasons I don't necessarily disagree with in general) above the activation energy of posting the patches, so I have no opinion either way; if I had to give one it'd fall under "sure, why not". |
|
I mean, I don't see how one could be tempted to think an aggregate-initialisation of any integer type to be nonzero, but if you feel like that's an issue that someone might fall victim to for some(?) reason, then sure. |
jwk404
added
Status: Accepted
In `zpool_load_compat()`:
* initialize `l_features[]` with a loop rather than a static
initializer.
* don't redefine system constants; use private names instead
Rationale here:
When an array is initialized using a static {foo}, only the specified
members are initialized to the provided values, the rest are
initialized to zero. While B_FALSE is of course zero, it feels
unsafe to rely on this being true forever, so I'm inclined to sacrifice
a few microseconds of runtime here and initialize using a loop.
When looking for the correct combination of system constants to use
(in open() and mmap()), I prefer to use private constants rather than
redefining system ones; due to the small chance that the system
ones might be referenced later in the file. So rather than defining
O_PATH and MAP_POPULATE, I use distinct constant names.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: John Kennedy <john.kennedy@delphix.com>
Signed-off-by: Colm Buckley <colm@tuatha.org>
Closes openzfs#12156
In `zpool_load_compat()`:
* initialize `l_features[]` with a loop rather than a static
initializer.
* don't redefine system constants; use private names instead
Rationale here:
When an array is initialized using a static {foo}, only the specified
members are initialized to the provided values, the rest are
initialized to zero. While B_FALSE is of course zero, it feels
unsafe to rely on this being true forever, so I'm inclined to sacrifice
a few microseconds of runtime here and initialize using a loop.
When looking for the correct combination of system constants to use
(in open() and mmap()), I prefer to use private constants rather than
redefining system ones; due to the small chance that the system
ones might be referenced later in the file. So rather than defining
O_PATH and MAP_POPULATE, I use distinct constant names.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: John Kennedy <john.kennedy@delphix.com>
Signed-off-by: Colm Buckley <colm@tuatha.org>
Closes openzfs#12156
In
zpool_load_compat():initialize
l_features[]with a loop rather than a staticinitializer.
don't redefine system constants; use private names instead
Rationale here:
When an array is initialized using a static {foo}, only the specified
members are initialized to the provided values, the rest are
initialized to zero. While B_FALSE is of course zero, it feels
unsafe to rely on this being true forever, so I'm inclined to sacrifice
a few microseconds of runtime here and initialize using a loop.
When looking for the correct combination of system constants to use
(in open() and mmap()), I prefer to use private constants rather than
redefining system ones; due to the small chance that the system
ones might be referenced later in the file. So rather than defining
O_PATH and MAP_POPULATE, I use distinct constant names.
Signed-off-by: Colm Buckley colm@tuatha.org